Compile benchmark with Dotty, fixes #29. #31

olafurpg · 2017-06-14T17:08:18Z

++0.2.0-RC1 to run benchmarks with dotc instead of scalac
compilation/test to quickly check the setup is OK without the need to remember cli args or start slow jmh process.

olafurpg · 2017-06-14T17:50:31Z

I'm not sure what exactly caused the CI failure, I suspect it's Travis killing the job.

lrytz

FOLLOW_LINKS is necessary when the corpusVersion parameter is not explicitly given through -p corpusVersion abcd123, in which case the value is "latest", which is a symlink (https://github.com/scala/compiler-benchmark/tree/master/corpus/better-files).

You say fork in run is required for running compilation/run, but the main way to execute benchmarks is through compilation/jmh:run HotScalacBenchmark, or even better by using the hot / cold aliases in sbt

I think you're right about the CI failure, it's probably a Travis timeout. For some reason the performance on travis is much lower than the previous successful run:

Before (https://travis-ci.org/scala/compiler-benchmark/builds/237376124)

[info] Iteration   1: 3221.225 ±(99.9%) 447.551 ms/op

Your run (https://travis-ci.org/scala/compiler-benchmark/builds/242931056)

[info] Iteration   1: 7541.359 ms/op

This might be due to a change on travis. Do you see comparable numbers locally?

lrytz · 2017-06-15T07:46:35Z

compilation/src/main/scala/scala/tools/nsc/ScalacBenchmark.scala

@@ -148,7 +126,7 @@ object ScalacBenchmarkStandalone {
 @BenchmarkMode(Array(SingleShotTime))
 @OutputTimeUnit(TimeUnit.MILLISECONDS)
 // TODO -Xbatch reduces fork-to-fork variance, but incurs 5s -> 30s slowdown
-@Fork(value = 16, jvmArgs = Array("-XX:CICompilerCount=2", "-Xms2G", "-Xmx2G"))
+//@Fork(value = 16, jvmArgs = Array("-XX:CICompilerCount=2", "-Xms2G", "-Xmx2G"))


we cannot "just" change this now, because the results after that change would probably be no longer comparable with the measurements we made so far.

Whoops, seems this did not get added to my latest push, I have staging commit

-//@Fork(value = 16, jvmArgs = Array("-XX:CICompilerCount=2", "-Xms2G", "-Xmx2G")) +@Fork(value = 16, jvmArgs = Array("-XX:CICompilerCount=2", "-Xms2G", "-Xmx2G")) class ColdScalacBenchmark extends ScalacBenchmark { @Benchmark def compile(): Unit = compileImpl() @@ -138,7 +138,7 @@ class ColdScalacBenchmark extends ScalacBenchmark { @Measurement(iterations = 1, time = 30, timeUnit = TimeUnit.SECONDS) // @Fork triggers match error in dotty, see https://github.com/lampepfl/dotty/issues/2704 // Comment out `@Fork` to run compilation/test with Dotty or wait for that issue to be fixed. -//@Fork(value = 3, jvmArgs = Array("-Xms2G", "-Xmx2G")) +@Fork(value = 3, jvmArgs = Array("-Xms2G", "-Xmx2G")) class WarmScalacBenchmark extends ScalacBenchmark { @Benchmark def compile(): Unit = compileImpl() @@ -148,7 +148,7 @@ class WarmScalacBenchmark extends ScalacBenchmark { @OutputTimeUnit(TimeUnit.MILLISECONDS) @Warmup(iterations = 10, time = 10, timeUnit = TimeUnit.SECONDS) @Measurement(iterations = 10, time = 10, timeUnit = TimeUnit.SECONDS) -//@Fork(value = 3, jvmArgs = Array("-Xms2G", "-Xmx2G")) +@Fork(value = 3, jvmArgs = Array("-Xms2G", "-Xmx2G")) class HotScalacBenchmark extends ScalacBenchmark { @Benchmark def compile(): Unit = compileImpl()

olafurpg

Thanks for the review @lrytz I updated the test to set corpusVersion and that fixed the FOLLOW_LINKS issue I encountered.

The // @Fork comments were not supposed to be there, although they are necessary at the moment to run compilation/test with Dotty.

While iterating on the build locally, I prefer to run compilation/test over hot -p ... since the jmh benchmarks are slow and Ctrl-c exits the sbt shell.

olafurpg · 2017-06-15T08:10:49Z

compilation/src/main/scala/scala/tools/nsc/ScalacBenchmark.scala

@@ -148,7 +126,7 @@ object ScalacBenchmarkStandalone {
 @BenchmarkMode(Array(SingleShotTime))
 @OutputTimeUnit(TimeUnit.MILLISECONDS)
 // TODO -Xbatch reduces fork-to-fork variance, but incurs 5s -> 30s slowdown
-@Fork(value = 16, jvmArgs = Array("-XX:CICompilerCount=2", "-Xms2G", "-Xmx2G"))
+//@Fork(value = 16, jvmArgs = Array("-XX:CICompilerCount=2", "-Xms2G", "-Xmx2G"))


Whoops, seems this did not get added to my latest push, I have staging commit

-//@Fork(value = 16, jvmArgs = Array("-XX:CICompilerCount=2", "-Xms2G", "-Xmx2G")) +@Fork(value = 16, jvmArgs = Array("-XX:CICompilerCount=2", "-Xms2G", "-Xmx2G")) class ColdScalacBenchmark extends ScalacBenchmark { @Benchmark def compile(): Unit = compileImpl() @@ -138,7 +138,7 @@ class ColdScalacBenchmark extends ScalacBenchmark { @Measurement(iterations = 1, time = 30, timeUnit = TimeUnit.SECONDS) // @Fork triggers match error in dotty, see https://github.com/lampepfl/dotty/issues/2704 // Comment out `@Fork` to run compilation/test with Dotty or wait for that issue to be fixed. -//@Fork(value = 3, jvmArgs = Array("-Xms2G", "-Xmx2G")) +@Fork(value = 3, jvmArgs = Array("-Xms2G", "-Xmx2G")) class WarmScalacBenchmark extends ScalacBenchmark { @Benchmark def compile(): Unit = compileImpl() @@ -148,7 +148,7 @@ class WarmScalacBenchmark extends ScalacBenchmark { @OutputTimeUnit(TimeUnit.MILLISECONDS) @Warmup(iterations = 10, time = 10, timeUnit = TimeUnit.SECONDS) @Measurement(iterations = 10, time = 10, timeUnit = TimeUnit.SECONDS) -//@Fork(value = 3, jvmArgs = Array("-Xms2G", "-Xmx2G")) +@Fork(value = 3, jvmArgs = Array("-Xms2G", "-Xmx2G")) class HotScalacBenchmark extends ScalacBenchmark { @Benchmark def compile(): Unit = compileImpl()

olafurpg

@lrytz my numbers locally

> hot -psource=scalap -p corpusVersion=latest -w1 -f1
...
[info] Iteration   1: 1705.334 ±(99.9%) 319.245 ms/op
...

olafurpg · 2017-06-15T08:31:48Z

compilation/src/main/scala/scala/tools/nsc/ScalacBenchmark.scala

-        true
-      }
+  def compilerArgs =
+    if (source.startsWith("@")) Array(source)


Note that dotc @foo.args is not yet supported, see scala/scala3#2759

lrytz · 2017-06-15T08:55:32Z

build.sbt

+  libraryDependencies += "com.novocode" % "junit-interface" % "0.11" % Test,
+  testOptions in Test += Tests.Argument(TestFrameworks.JUnit),
+  fork in (Test, test) := true,
+  fork in run := true


could you add a comment here why fork is needed? what's the use case for compilation/run anyway?

I removed fork in run := true and added a brief comment explaining fork in (Test, test) := true. I can leave out the unit test from this PR if you prefer. I personally find it useful to be able to run ~compilation/test while iterating on the code. My first intuition in an unfamiliar sbt build is usually to run test.

Thanks! It's fine for me to leave it in. I'll let @retronym take a look at this PR too, to me it looks very good!

dragos · 2017-07-06T09:53:16Z

build.sbt

-  libraryDependencies += "org.scala-lang" % "scala-compiler" % scalaVersion.value,
-  mainClass in (Jmh, run) := Some("scala.bench.ScalacBenchmarkRunner")
+  libraryDependencies += {
+    if (isDotty.value) "ch.epfl.lamp" %% "dotty-compiler" % scalaVersion.value


I think it would be good to also use Sbt's scalaOrganization, this would allow the Typelevel compiler to "just work" (and others too :D).

Does that help when the artifact id is dotty-compiler?

Nope, I opened #34 myself.

biboudis · 2017-07-11T14:31:16Z

scala/scala3#2819 is now merged!

Note, the benchmarks currently don't actually run with Dotty because of scala/scala3#2704. - When `scalaVersion := "0.x"`, then "src/main/dotc" is added to unmanagedSourceDirectories, otherwise "src/main/scalac" is added. - compilation/test checks the setup is OK without the need to remember cli args. - `fork in run` is necessary for -usejavacp to work with compilation/run - `FileVisitOption.FOLLOW_LINKS` produces duplicate filenames, which causes "X is already compiled in this run" errors in Dotty.

This option seems to be necessary to run the JMH benchmarks. This change causes the Dotty tests to fail with the error: "object Vector in package immutable has already been compiled once during this run".

This prevents the "object vector is already compiled in this run" error when testing the Dotty compilation.

olafurpg · 2017-07-11T16:07:54Z

I'll try to rebase on master tomorrow and upgrade to Dotty 0.2.0-RC1.

olafurpg · 2017-07-12T09:20:17Z

CI is green but the logs show a NPE: https://travis-ci.org/scala/compiler-benchmark/builds/252729442#L840

java.lang.NullPointerException
[info] 	at scala.reflect.internal.util.BatchSourceFile.<init>(SourceFile.scala:111)
[info] 	at scala.tools.nsc.TreeTraverserBenchmark.setup(TreeTraverserBenchmark.scala:30)
[info] 	at scala.tools.nsc.generated.TreeTraverserBenchmark_measure_jmhTest._jmh_tryInit_f_treetraverserbenchmark0_0(TreeTraverserBenchmark_measure_jmhTest.java:338)
[info] 	at

Should we change the CI to test dotc and scalac compilation?

olafurpg · 2017-07-12T09:31:01Z

The NPE is not a regression from this PR, I can reproduce it on master branch and the Travis logs show the same https://travis-ci.org/scala/compiler-benchmark/builds/250716575#L820

I've updated .travis.yml to test compilation for scalac and dotc.

The micro/jmh command causes a NPE exception which kills the sbt process with an exit code 0. This caused CI to report green even if the compilation/test command did not run.

olafurpg · 2017-07-12T10:38:17Z

The CI now runs compilation/test for scalac and dotc, see https://travis-ci.org/scala/compiler-benchmark/builds/252748994#L920. This PR is ready for another review.

biboudis · 2017-07-12T10:49:41Z

Still NPE, no?

https://travis-ci.org/scala/compiler-benchmark/builds/252748994#L1027

olafurpg · 2017-07-12T10:58:25Z

@biboudis Yes, but the NPE is unrelated to this PR. It appears as well in the logs on master branch https://travis-ci.org/scala/compiler-benchmark/builds/250716575#L783

lrytz · 2017-07-12T11:00:06Z

Ah right, we should fix that.

lrytz · 2017-07-12T10:54:23Z

.travis.yml

@@ -3,7 +3,7 @@ language: scala
 env:
  global:

-script: sbt test:compile "hot -psource=scalap -w1 -f1" "micro/jmh:run -w1 -f1"
+script: sbt test:compile "hot -psource=scalap -w1 -f1" "; project compilation ; +test" "micro/jmh:run -w1 -f1"


So we're running test:compile and hot... only on scalac for now because @Fork still crashes dotc, right?

The @Fork annotation issue has been fixed, I can change the CI to run dotty benchmarks in CI too.

There's still a comment in the diff that links to scala/scala3#2704

I've removed the comment, the issue has been fixed and should be closed.

lrytz · 2017-07-12T10:57:38Z

compilation/src/main/dotc/scala/tools/benchmark/BenchmarkDriver.scala

+    ctx.setSetting(ctx.settings.d, tempDir.getAbsolutePath)
+    ctx.setSetting(ctx.settings.language, List("Scala2"))
+    val compiler = new dotty.tools.dotc.Compiler
+    val args = compilerArgs ++ sourceFiles


should we also process extraArgs here?

The long sbt command in .travis.yml was getting unwieldy. This commit adds a `testAll` command which can easily be run locally from the sbt shell to test the command executed in CI.

olafurpg · 2017-07-12T11:20:43Z

I took the liberty to move the long sbt command from .travis.yml into build.sbt. This makes it easier to reproduce the CI command locally from the sbt shell.

olafurpg · 2017-07-12T11:26:10Z

build.sbt

+commands += Command.command("testAll") { s =>
+  "test:compile" ::
+    "compilation/test" ::
+    "hot -psource=scalap -wi 1 -i 1 -f1" ::


Note. I bumped down the warmup iterations from 10 to 1 to make the tests run faster. The benchmark numbers on Travis are too unreliable anyways.

lrytz · 2017-07-12T12:46:07Z

build.sbt

+commands += Command.command("testAll") { s =>
+  "test:compile" ::
+    "compilation/test" ::
+    "hot -psource=scalap -wi 1 -i 1 -f1" ::


lrytz · 2017-07-12T12:48:18Z

build.sbt

+
+commands += Command.command("testAll") { s =>
+  "test:compile" ::
+    "compilation/test" ::


should this be done on dotty too?

It is done in Dotty as well, see L14.

+compilation/test would not run on Dotty because it uses crossScalaVersions from the root project. Alternatives are

project compilation; +test ; project compiler-benchmark

add the sbt-doge plugin and run such compilation/test

add 0.2.0-RC1 to compiler-benchmark/crossScalaVersions, but that would be incorrect.

I opted for duplication since it's the dumbest solution.

lrytz

Looks good, thanks a lot for your work! I'll leave it open for @retronym (he's out this week).

olafurpg · 2017-07-12T13:14:19Z

Cool. I leave for vacation next week Wednesday so hopefully we can get this in before then :)

olafurpg · 2017-08-15T12:39:36Z

Ping. Any update on this?

olafurpg · 2017-08-23T06:29:24Z

Any estimate when this will get reviewed?

retronym

LGTM, thanks a lot a sorry about the slow review.

olafurpg · 2017-08-23T07:36:49Z

Thank you, no worries 😄

retronym · 2017-08-23T09:12:50Z

After #38, I'm able to run benchmark the vector corpus with Dotty 0.2.0-RC1.

Trying better-files results in compile errors. scalap gives a compiler crash. Details. Known problems?

retronym · 2017-08-23T09:18:04Z

Trying 0.3.0-RC1 now, seems the same.

retronym · 2017-08-23T09:39:20Z

better-files compiles with 2.12.3 and Dottty after removing a few troublesome lines of code.

diff --git a/compilation/src/main/dotc/scala/tools/benchmark/BenchmarkDriver.scala b/compilation/src/main/dotc/scala/tools/benchmark/BenchmarkDriver.scala
index 6047a33..632210d 100644
--- a/compilation/src/main/dotc/scala/tools/benchmark/BenchmarkDriver.scala
+++ b/compilation/src/main/dotc/scala/tools/benchmark/BenchmarkDriver.scala
@@ -12,7 +12,7 @@ trait BenchmarkDriver extends BaseBenchmarkDriver {
       ctx.setSetting(ctx.settings.classpath,
                      depsClasspath.mkString(File.pathSeparator))
     }
-    ctx.setSetting(ctx.settings.migration, true)
+    ctx.setSetting(ctx.settings.migration, false)
     ctx.setSetting(ctx.settings.d, tempDir.getAbsolutePath)
     ctx.setSetting(ctx.settings.language, List("Scala2"))
     val compiler = new dotty.tools.dotc.Compiler
diff --git a/corpus/better-files/a45f905/File.scala b/corpus/better-files/a45f905/File.scala
index 8f46b00..876f5e1 100644
--- a/corpus/better-files/a45f905/File.scala
+++ b/corpus/better-files/a45f905/File.scala
@@ -264,7 +264,7 @@ class File private(val path: Path) {
   }
 
   def writeBytes(bytes: Iterator[Byte])(implicit openOptions: File.OpenOptions = File.OpenOptions.default): this.type = {
-    outputStream(openOptions).foreach(_.buffered write bytes)
+    ??? // outputStream(openOptions).foreach(x => x.buffered.write(bytes))
     this
   }
 
diff --git a/corpus/better-files/a45f905/Implicits.scala b/corpus/better-files/a45f905/Implicits.scala
index a9d26a6..ef36739 100644
--- a/corpus/better-files/a45f905/Implicits.scala
+++ b/corpus/better-files/a45f905/Implicits.scala
@@ -180,7 +180,7 @@ trait Implicits {
         f(resource)
       } finally {
         if (!isClosed) {
-          resource.close()
+          ???
           isClosed = true
         }
       }
@@ -207,7 +207,7 @@ trait Implicits {
       }
 
       def close() = try {
-        if (!isClosed) resource.close()
+        ??? // if (!isClosed) resource.close()
       } finally {
         isClosed = true
       }
@@ -218,7 +218,7 @@ trait Implicits {
         next
       } catch {
         case NonFatal(ex) =>
-          close()
+          ??? // close()
           throw ex
       }
 
diff --git a/corpus/better-files/a45f905/Scanner.scala b/corpus/better-files/a45f905/Scanner.scala
index 9295e54..ec7095c 100644
--- a/corpus/better-files/a45f905/Scanner.scala
+++ b/corpus/better-files/a45f905/Scanner.scala
@@ -57,7 +57,7 @@ object Scanner {
     */
   case class Config(delimiter: String, includeDelimiters: Boolean)(implicit val codec: Codec)
   object Config {
-    implicit val default = Config(delimiter = Delimiters.whitespaces, includeDelimiters = false)
+    implicit val default: Config = Config(delimiter = Delimiters.whitespaces, includeDelimiters = false)
 
     object Delimiters {
       val lines = "\n\r"
diff --git a/corpus/better-files/a45f905/package.scala b/corpus/better-files/a45f905/package.scala
index 8ef79e8..9edf2e4 100644
--- a/corpus/better-files/a45f905/package.scala
+++ b/corpus/better-files/a45f905/package.scala
@@ -17,7 +17,8 @@ package object files extends Implicits {
   @inline private[files] def when[A](condition: Boolean)(f: => A): Option[A] = if (condition) Some(f) else None
   @inline private[files] def repeat(n: Int)(f: => Unit): Unit = (1 to n).foreach(_ => f)
 
-  private[files] def produce[A](f: => A) = new {
+  private[files] def produce[A](f: => A): RichAny[A] = new RichAny(f)
+  private[files] class RichAny[A](f: => A) {
     def till(hasMore: => Boolean): Iterator[A] = new Iterator[A] {
       override def hasNext = hasMore
       override def next() = f

scalac compile times are is 0.65x that of 0.3.0-RC1. I understand the dotty build isn't able to eliminate debug tracing code that wraps a lot of core operations, this needs some search/replace. Anyway, it would be good to get an optimized build published so we can remove that noise.

olafurpg · 2017-08-23T09:47:04Z

Dotty still fails to compile some Scala 2.x programs, most of the time it's due to known incompatibilities (missing type annotation for public implicit definition or scala.reflect macros). Sometimes it's just bugs.

I'm not involved in dotty performance optimizations, I think it's best you bring that up with the Dotty team to get a -optimize compiled release.

BTW, note that intellij-scala has a known issues when loading sbt projects with scalaVersion := "0.x" https://youtrack.jetbrains.com/oauth?state=%2Fissue%2FSCL-12237

olafurpg mentioned this pull request Jun 14, 2017

benchmark dotty compiler #29

Closed

lrytz reviewed Jun 15, 2017

View reviewed changes

olafurpg commented Jun 15, 2017

View reviewed changes

olafurpg mentioned this pull request Jun 15, 2017

Match error in backend using Java defined annotation with default parameters scala/scala3#2704

Closed

lrytz reviewed Jun 15, 2017

View reviewed changes

This was referenced Jun 23, 2017

Assertion error in DottyBackendInterface: denot.name is not default getter scala/scala3#2797

Closed

Fix #2797: Don't lift Java annotation arguments out of call scala/scala3#2819

Merged

dragos reviewed Jul 6, 2017

View reviewed changes

olafurpg added 5 commits July 11, 2017 17:30

Re-enable FileVisitOption.FOLLOW_LINKS.

16fe81a

This option seems to be necessary to run the JMH benchmarks. This change causes the Dotty tests to fail with the error: "object Vector in package immutable has already been compiled once during this run".

Uncomment @Fork.

1bd6dd5

Set corpusVersion in compilation/test.

ccc6b38

This prevents the "object vector is already compiled in this run" error when testing the Dotty compilation.

Explain fork in (Test, test) := true.

49b2fb0

olafurpg added 2 commits July 12, 2017 11:01

Fix rebase on master.

ef391dd

Fix -migration warning.

1a975a9

olafurpg force-pushed the dotty2 branch from c306103 to 1a975a9 Compare July 12, 2017 09:07

Test dotty compilation in CI.

88190e1

olafurpg mentioned this pull request Jul 12, 2017

Update version to 0.3.0 scala/scala3#2852

Merged

Change order of tests on CI.

6ed08f2

The micro/jmh command causes a NPE exception which kills the sbt process with an exit code 0. This caused CI to report green even if the compilation/test command did not run.

lrytz reviewed Jul 12, 2017

View reviewed changes

olafurpg added 3 commits July 12, 2017 13:18

Run dotty jmh benchmarks in CI.

ab1080a

The long sbt command in .travis.yml was getting unwieldy. This commit adds a `testAll` command which can easily be run locally from the sbt shell to test the command executed in CI.

Reuse extraArgs logic for dotc and scalac.

4f27da1

Remove outdated comment, the annotation issue has been fixed

0a937bb

olafurpg commented Jul 12, 2017

View reviewed changes

Reuse scala211 and dottyLatest variables in testAll.

dd00264

lrytz reviewed Jul 12, 2017

View reviewed changes

biboudis mentioned this pull request Aug 15, 2017

Dotty benchmarks scala/scala3#2307

Closed

7 tasks

retronym approved these changes Aug 23, 2017

View reviewed changes

retronym merged commit 6989dd9 into scala:master Aug 23, 2017

olafurpg deleted the dotty2 branch August 23, 2017 07:36

smarter mentioned this pull request Sep 5, 2017

Native applied types scala/scala3#3033

Closed

Compile benchmark with Dotty, fixes #29. #31

Compile benchmark with Dotty, fixes #29. #31

Conversation

olafurpg commented Jun 14, 2017 • edited Loading

olafurpg commented Jun 14, 2017

lrytz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

biboudis commented Jul 11, 2017 • edited Loading

olafurpg commented Jul 11, 2017

olafurpg commented Jul 12, 2017

olafurpg commented Jul 12, 2017

olafurpg commented Jul 12, 2017

biboudis commented Jul 12, 2017

olafurpg commented Jul 12, 2017

lrytz commented Jul 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg commented Jul 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lrytz left a comment

Choose a reason for hiding this comment

olafurpg commented Jul 12, 2017

olafurpg commented Aug 15, 2017

olafurpg commented Aug 23, 2017

retronym left a comment

Choose a reason for hiding this comment

olafurpg commented Aug 23, 2017

retronym commented Aug 23, 2017

retronym commented Aug 23, 2017

retronym commented Aug 23, 2017

olafurpg commented Aug 23, 2017

olafurpg commented Jun 14, 2017 •

edited

Loading

biboudis commented Jul 11, 2017 •

edited

Loading

olafurpg commented Jul 12, 2017 •

edited

Loading